Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add functions to find optimal partitions using ILP solvers. #3

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

erelsgl
Copy link

@erelsgl erelsgl commented Oct 18, 2021

Also added a "demo" folder with some simple demo programs.

such as Karmarkar-Karp and Greedy.
"""

from numberpartitioning import karmarkar_karp, greedy, complete_greedy
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be useful if the examples here were included as docstrings on the relevant functions or modules. This way, they would be easy to include in auto-generated documentation, and it would be possible to run them as tests locally and in CI.

E.g. something like the example sections in the methods in https://github.com/scipy/scipy/blob/master/scipy/sparse/csgraph/_flow.pyx works well.


import cvxpy

DEFAULT_SOLVERS=[cvxpy.XPRESS, cvxpy.OSQP, cvxpy.SCS]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this require that the relevant binaries are available on the user's machine? And at least one of them requires a license for large problem instances.

Allowing to use these solvers is fine, but it would then be nice if all relevant dependencies were available through pypi or conda, so it's possible to use the library in a self-contained fashion.

Those would then be specifies as optional dependencies for the Python package.

except cvxpy.SolverError as err:
logger.info("Solver %s fails: %s", solver, err)
if not is_solved:
problem.solve(solver=solvers[-1]) # If the first n-1 fail, try the last one.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it should be possible to treat the final solver alongside the other ones. Any reason it's handled specifically?


Returns
-------
A partition representing by a ``PartitioningResult``.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this always runs to optimality right? Given the NP-hard nature of the problem, that might be a bit of an ask. Would it be a lot of work to use callbacks to generate solutions along the way, or maybe make it easy to allow it to return non-optimak results (through time constraints, allowed gap, etc.)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code uses an existing ILP solver, so - as far as I know - it cannot return solutions along the way. On the positive side, ILP solvers use many tricks that make the solution very efficient in practice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants